[msbuild] Show an error if SdkDevPath hasn't been set and we're executing a process. #24555
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request adds validation to detect when an MSBuild task tries to execute an external process without having the SdkDevPath property set. This helps catch configuration errors earlier with a clear error message.
Changes:
- Added error detection in
ExecuteAsyncto log an error when SdkDevPath is not set on macOS - Updated test infrastructure to automatically set SdkDevPath when creating XamarinTask instances
- Added new error message E7164 to the localization resources
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| msbuild/Xamarin.MacDev.Tasks/Tasks/XamarinTask.cs | Added validation to check for missing SdkDevPath before executing external processes, and changed ExecuteAsync signature to accept Task instead of TaskLoggingHelper |
| tests/msbuild/Xamarin.MacDev.Tasks.Tests/TestHelpers/TestBase.cs | Updated CreateTask method to automatically set SdkDevPath for XamarinTask instances to prevent test failures |
| msbuild/Xamarin.Localization.MSBuild/MSBStrings.resx | Added new error message E7164 for missing SdkDevPath scenario |
|
|
||
| if (Environment.OSVersion.Platform == PlatformID.MacOSX && string.IsNullOrEmpty (sdkDevPath)) { | ||
| log.LogError (MSBStrings.E7164 /* The task '{0}' is trying to call an external process, but a path to Xcode has not been provided. Please file an issue at https://github.com/dotnet/macios/issues/new/choose. */, task.GetType ().Name); | ||
| log.LogMessage (MessageImportance.Low, Environment.StackTrace); |
There was a problem hiding this comment.
Logging Environment.StackTrace here will show the stack from where Environment class is called, not the actual call stack leading to this error. Consider using new StackTrace().ToString() instead to get the actual call stack, which would be more useful for debugging.
|
|
||
| if (Environment.OSVersion.Platform == PlatformID.MacOSX && string.IsNullOrEmpty (sdkDevPath)) { | ||
| log.LogError (MSBStrings.E7164 /* The task '{0}' is trying to call an external process, but a path to Xcode has not been provided. Please file an issue at https://github.com/dotnet/macios/issues/new/choose. */, task.GetType ().Name); | ||
| log.LogMessage (MessageImportance.Low, Environment.StackTrace); |
There was a problem hiding this comment.
After logging the error about the missing SdkDevPath, the code continues to execute the external process, which will likely fail or produce confusing errors. Consider returning early after logging the error to prevent cascading failures and make the error message clearer. This could be done by returning a failed Execution object or throwing an exception that gets caught by the caller.
| log.LogMessage (MessageImportance.Low, Environment.StackTrace); | |
| log.LogMessage (MessageImportance.Low, Environment.StackTrace); | |
| throw new InvalidOperationException ("The task is trying to call an external process, but a path to Xcode has not been provided."); |
✅ [CI Build #018c9a4] Build passed (Build packages) ✅Pipeline on Agent |
✅ [PR Build #018c9a4] Build passed (Detect API changes) ✅Pipeline on Agent |
✅ API diff for current PR / commitNET (empty diffs)✅ API diff vs stableNET (empty diffs)ℹ️ Generator diffGenerator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes) Pipeline on Agent |
✅ [CI Build #018c9a4] Build passed (Build macOS tests) ✅Pipeline on Agent |
💻 [CI Build #018c9a4] Tests on macOS X64 - Mac Sonoma (14) passed 💻✅ All tests on macOS X64 - Mac Sonoma (14) passed. Pipeline on Agent |
💻 [CI Build #018c9a4] Tests on macOS M1 - Mac Monterey (12) passed 💻✅ All tests on macOS M1 - Mac Monterey (12) passed. Pipeline on Agent |
💻 [CI Build #018c9a4] Tests on macOS M1 - Mac Ventura (13) passed 💻✅ All tests on macOS M1 - Mac Ventura (13) passed. Pipeline on Agent |
💻 [CI Build #018c9a4] Tests on macOS arm64 - Mac Sequoia (15) passed 💻✅ All tests on macOS arm64 - Mac Sequoia (15) passed. Pipeline on Agent |
💻 [CI Build #018c9a4] Tests on macOS arm64 - Mac Tahoe (26) passed 💻✅ All tests on macOS arm64 - Mac Tahoe (26) passed. Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
🚀 [CI Build #018c9a4] Test results 🚀Test results✅ All tests passed on VSTS: test results. 🎉 All 124 tests passed 🎉 Tests counts✅ cecil: All 1 tests passed. Html Report (VSDrops) Download Pipeline on Agent |
No description provided.